-
-
Notifications
You must be signed in to change notification settings - Fork 15
Implement formatting check for long hyphen #261
Implement formatting check for long hyphen #261
Conversation
Fix these ./tor/validation/formatting_validation.py:125:1: E302 expected 2 blank lines, found 1 ./tor/validation/formatting_validation.py:130:1: W293 blank line contains whitespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation itself looks good, however we also need to define the response string for this formatting issue in tor/strings/en_US.yml
. For examples you can take a look at the other formatting issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to define the corresponding strings for the bot response.
Here we still need to define the bot response for the volunteers, you can take https://github.com/GrafeasGroup/tor/blob/main/tor/strings/en_US.yml#L209-L319 as examples :) |
""" | ||
return ( | ||
FormattingIssue.AUTOGENERATED_LONG_HYPHEN | ||
if AUTOGENERATED_LONG_HYPHEN_PATTERN.search(transcription) is not None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not drop the regex and do a simple x in y
check?
if AUTOGENERATED_LONG_HYPHEN_PATTERN.search(transcription) is not None | |
if "\u2014" in transcription # \u2014 == em dash == "long hyphen" | |
], | ||
) | ||
def test_check_for_autogenerated_long_hyphen(test_input: str, should_match: bool) -> None: | ||
"""Test if autogenerated long hyphens are detected.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The long hyphen isn't always auto-generated, so perhaps "accidental" would be a better descriptor than "autogenerated"?
Side note: if settings for "smart quotes" (and the like) are enabled on macOS, it automatically replaces --
with —
there too. Not just mobile. 😄
No comments and no meaningful code updates in months. Closing this since it seems pretty stale to me. Feel free to reopen if the mood strikes to pick this up again! |
Not too necessary since missing separator check will detect it as well